metis: implement gRPC daemon server#1024
metis: implement gRPC daemon server#1024YifeiZhuang wants to merge 12 commits intokubernetes:masterfrom
Conversation
|
Skipping CI for Draft Pull Request. |
k8s-ci-robot
added
do-not-merge/work-in-progress
|
This issue is currently awaiting triage. If the repository mantainers determine this is a relevant issue, they will accept it by applying the The DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
k8s-ci-robot
added
approved
|
/test |
|
/test all |
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
YifeiZhuang
force-pushed
the
impl-daemon-server
branch
from
f8b194b to
f779101
Compare
|
@arvindbr8 @gnossen @zhaoqsh PTAL |
arvindbr8
left a comment
arvindbr8
left a comment
There was a problem hiding this comment.
Thanks @YifeiZhuang for taking care of this. I have done an initial pass and had a few comments.
| t.Fatalf("AddCIDR failed: %v", err) | ||
| } | ||
|
|
||
| const numGoroutines = 50 // High contention |
There was a problem hiding this comment.
Since we capped MaxOpenConns at 10 on the DB pool, won't Go just throttle these 50 goroutines? I'm wondering if this will actually test 50 concurrent SQLite operations, or if it just tests 10 at a time while the other 40 sit in Go's connection queue.
There was a problem hiding this comment.
The first 10 goroutines will get initial connections. For the remaining they will be waiting for an until other execution is paused. It will not throttle or error. It looks to me the right number of goroutines that we can test for. I've verified the sqlite waiting stats.
wg.Wait()
// Retrieve DB stats
stats := s.DB().Stats()
t.Logf("DB Stats after high contention: %+v", stats)
// Verify that at least some goroutines had to wait for a connection
if stats.WaitCount == 0 {
t.Errorf("Expected some goroutines to wait for connections, but WaitCount was 0")
} else {
t.Logf("Success: %d requests had to wait for a connection.", stats.WaitCount)
}
you will likely need to rebase to master to pull in changes from #1036 |
|
@YifeiZhuang a |
I added to the tool: #1038, no wonder it does not fail verify-up-to-date. |
arvindbr8
left a comment
arvindbr8
left a comment
There was a problem hiding this comment.
LGTM. thanks Ivy, this is great!
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: arvindbr8, YifeiZhuang The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@YifeiZhuang: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
4 participants
Key changes include:
adaptiveipamgRPC server (daemon_server.go) to listen for pod IP allocation requests over a Unix Domain Socket. And implement rpcAllocatePodIPandDeallocatePodIP.Storeto interface with the SQLite DB, supporting idempotency for all:AddCIDR: Add CIDR blocks and seeds individual IP addresses.AllocateIPv4: Find available IP slots and flipsis_allocatedto true.ReleaseIPByOwner: Releases pod IP addresses by owner identifiers and sets cooldown period timestamp.Threading model:
storeconcurrently to optimize request latency. The DB transactions guarantees thread safet between concurrent requests. Existing WAL mode andbusy_timeoutsupports high concurrent read/write operations without locking.No implementation for IPv6 yet.
/label tide/merge-method-squash